Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement SplitInstallService #2500

Open
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

DaVinci9196
Copy link
Contributor

Fix maps language modification and Fix Smart Lens translation language download

@CoelacanthusHex
Copy link
Contributor

Will it fix #1629 and partially resolve #2225?

@mar-v-in
Copy link
Member

This PR seems to be mixing completely unrelated features (Phenotype / Experiment Flags and SplitInstallService). I don't think SplitInstallService strictly needs Phenotype related functionality. Please clean this up so the PR only contains the SplitInstallService

@DaVinci9196
Copy link
Contributor Author

This PR seems to be mixing completely unrelated features (Phenotype / Experiment Flags and SplitInstallService). I don't think SplitInstallService strictly needs Phenotype related functionality. Please clean this up so the PR only contains the SplitInstallService

I submitted a new revision

vending-app/src/main/AndroidManifest.xml Outdated Show resolved Hide resolved
vending-app/src/main/AndroidManifest.xml Outdated Show resolved Hide resolved
Comment on lines 62 to 216
context,
SettingsContract.CheckIn.getContentUri(context),
arrayOf(SettingsContract.CheckIn.ANDROID_ID)
) { cursor: Cursor -> cursor.getLong(0) }

millis = System.currentTimeMillis()
timestamp
.container1Wrapper(
TimestampContainer1Wrapper.Builder()
.androidId(androidId.toString())
.container(
TimestampContainer1.Builder()
.timestamp(millis.toString() + "000")
.wrapper(makeTimestamp(millis))
.build()
)
.build()
)
val encodedTimestamps = String(
Base64.encode(
Util.encodeGzip(timestamp.build().encode()),
Base64.URL_SAFE or Base64.NO_WRAP or Base64.NO_PADDING
)
)
val locality = Locality.Builder()
.unknown1(1)
.unknown2(2)
.countryCode("")
.region(
TimestampStringWrapper.Builder()
.string("")
.timestamp(makeTimestamp(System.currentTimeMillis())).build()
)
.country(
TimestampStringWrapper.Builder()
.string("")
.timestamp(makeTimestamp(System.currentTimeMillis())).build()
)
.unknown3(0)
.build()
val encodedLocality = String(
Base64.encode(locality.encode(), Base64.URL_SAFE or Base64.NO_WRAP or Base64.NO_PADDING)
)
val header = LicenseRequestHeader.Builder()
.encodedTimestamps(StringWrapper.Builder().string(encodedTimestamps).build())
.triple(
EncodedTripleWrapper.Builder().triple(
EncodedTriple.Builder()
.encoded1("")
.encoded2("")
.empty("")
.build()
).build()
)
.locality(LocalityWrapper.Builder().encodedLocalityProto(encodedLocality).build())
.unknown(IntWrapper.Builder().integer(5).build())
.empty("")
.languages(externalxpsrh)
.deviceMeta(
DeviceMeta.Builder()
.android(
AndroidVersionMeta.Builder()
.androidSdk(org.microg.gms.profile.Build.VERSION.SDK_INT)
.buildNumber(org.microg.gms.profile.Build.ID)
.androidVersion(org.microg.gms.profile.Build.VERSION.RELEASE)
.unknown(0)
.build()
)
.unknown1(UnknownByte12.Builder().bytes(ByteString.EMPTY).build())
.unknown2(1)
.build()
)
.userAgent(
UserAgent.Builder()
.deviceName(org.microg.gms.profile.Build.DEVICE)
.deviceHardware(org.microg.gms.profile.Build.HARDWARE)
.deviceModelName(org.microg.gms.profile.Build.MODEL)
.finskyVersion(FINSKY_VERSION)
.deviceProductName(org.microg.gms.profile.Build.MODEL)
.androidId(androidId) // must not be 0
.buildFingerprint(org.microg.gms.profile.Build.FINGERPRINT)
.build()
)
.uuid(
Uuid.Builder()
.uuid(UUID.randomUUID().toString())
.unknown(2)
.build()
)
.build().encode()
return String(Base64.encode(Util.encodeGzip(header), Base64.URL_SAFE or Base64.NO_WRAP or Base64.NO_PADDING))
}

private fun getHeaders(): Map<String, String> {
headerMap["X-PS-RH"] = getXHeaders()
headerMap["Authorization"] = "Bearer " + AccountManager.get(context).getAuthToken(
user, tokenType, null, false, null, null
).result.getString(AccountManager.KEY_AUTHTOKEN)
return this.headerMap
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to mostly overlap with getLicenseRequestHeaders in LicenseRequestHeaders.kt. Maybe you can refactor this into a common codebase.

@mar-v-in mar-v-in added this to the 0.3.4 milestone Aug 27, 2024
@DaVinci9196 DaVinci9196 deleted the split_install_service branch August 27, 2024 11:48
@DaVinci9196 DaVinci9196 restored the split_install_service branch August 27, 2024 11:48
@DaVinci9196 DaVinci9196 reopened this Aug 27, 2024
@DaVinci9196 DaVinci9196 changed the title SplitInstallService Additions Implement AssetModuleService Aug 27, 2024
@DaVinci9196 DaVinci9196 changed the title Implement AssetModuleService Implement SplitInstallService Aug 27, 2024
@DaVinci9196
Copy link
Contributor Author

We modified it according to your opinion

Copy link
Contributor

@fynngodau fynngodau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've had a look at the code and provide some suggestions for improvement. Note that I haven't tested the changes and thus can't tell whether they are working as intended.

Comment on lines 196 to 203
get(url = requestUrl, headers = getLicenseRequestHeaders(auth, 1).toMutableMap().apply {
val xPsRh = String(
Base64.encode(
getDefaultLicenseRequestHeaderBuilder(1).languages(RequestLanguagePackage.Builder().language(requestLanguagePackage).build()).build().encode().encodeGzip(),
Base64.URL_SAFE or Base64.NO_WRAP or Base64.NO_PADDING
)
)
put("X-PS-RH", xPsRh)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see no reason why adding the language field should not be integrated with the code LicenseRequestHeaders (perhaps as an optional parameter). Then you wouldn't have to calculate the X-PS-RH value again here. Also, probably the file and methods in LicenseRequestHeaders should be renamed and pulled out of the .license package, considering they are now not only used for license checking (as well as renaming the corresponding proto files).

@@ -28,4 +28,5 @@
<string name="tips_forget_passwd">Forget password?</string>
<string name="tips_more_details">Learn more</string>
<string name="text_verify_button">Verify</string>
<string name="split_install">%s Downloading subpackages</string>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not very descriptive (and also not well-formatted). I would suggest

Suggested change
<string name="split_install">%s Downloading subpackages</string>
<string name="split_install">Downloading required components for %s</string>

sendCompleteBroad(context, intent)
}

private suspend fun getDownloadUrls(context: Context, httpClient: HttpClient, packageName: String, langName: Array<String>, splitName: Array<String>): ArrayList<Array<String>> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would change the return type to either List<Pair<String,String>> or even List<DownloadUrl> with DownloadUrl as either a typealias to Pair<String,String> or as a separate type.

You can then make the code that adds to the list easier to read using a to b instead of arrayOf(a, b), and the compiler guarantees that each item has exactly two values.

Comment on lines 81 to 82
if (lastSplitPackageName != null && lastSplitPackageName != pkg) {
if (mutex.isLocked) mutex.unlock()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When does this case occur?


private suspend fun installSplitPackage(context: Context, httpClient: HttpClient, downloadUrl: ArrayList<Array<String>>, packageName: String, language: String?): Intent {
Log.d(TAG, "installSplitPackage downloadUrl: ${downloadUrl.firstOrNull()}")
if (Build.VERSION.SDK_INT < Build.VERSION_CODES.LOLLIPOP) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This check is only executed after calling getDownloadUrls. I would suggest performing the check earlier in the flow, and annotating relevant methods with @RequiresApi(Build.VERSION_CODES.LOLLIPOP).

Comment on lines 47 to 52
override fun startInstall(pkg: String, splits: List<Bundle>, bundle0: Bundle, callback: ISplitInstallServiceCallback) {
Log.d(TAG, "Method <startInstall> Called by package: $pkg")
lifecycleScope.launch {
trySplitInstall(context, httpClient, pkg, splits)
Log.d(TAG, "onStartInstall SUCCESS")
callback.onStartInstall(CommonStatusCodes.SUCCESS, Bundle())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems to me like the identity of the caller is not verified. I would expect packages are only allowed to request split installs for themselves, right? We should ensure that pkg is indeed the caller themselves by calling PackageUtils.getAndCheckCallingPackage.

@DaVinci9196
Copy link
Contributor Author

@fynngodau Accepted, thanks for the suggestion.
In practice, the packageName provided by the calling method cannot be verified by PackageUtils.getAndCheckCallingPackage to be the same calling

@fynngodau
Copy link
Contributor

fynngodau commented Sep 14, 2024

Thanks for providing us with a new iteration!

I now noticed that HeaderProvider and com.android.vending.extensions are partially overlapping and duplicate, and the latter is not well-integrated with the classes (AuthData and DeviceEnvInfo) used by the former. Would it be possible to harmonize and merge the two files? Though I see the difficulty considering that some calls seem to want an x-ps-rh header and others do not.

Is there a reason you are using 1 instead of the real (microG) Android ID in your queries?

In practice, the packageName provided by the calling method cannot be verified by PackageUtils.getAndCheckCallingPackage to be the same calling

I can't see why it wouldn't work the way you tried, but maybe @mar-v-in has an idea.

val intent = Intent(context, InstallResultReceiver::class.java).apply {
putExtra(KEY_BYTES_DOWNLOADED, totalDownloaded)
}
val pendingIntent = PendingIntent.getBroadcast(context, sessionId, intent, 0)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
val pendingIntent = PendingIntent.getBroadcast(context, sessionId, intent, 0)
val pendingIntent = PendingIntent.getBroadcast(context, sessionId, intent, PendingIntent.FLAG_UPDATE_CURRENT | PendingIntent.FLAG_IMMUTABLE)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jonathanklee With FLAG_IMMUTABLE, I am not receiving any response values in the InstallResultReceiver, so the receiver can't detect if installation was successful or not. So FLAG_MUTABLE seems better.

private suspend fun requestDownloadUrls(callingPackage: String, authToken: String, packs: MutableSet<String>): ArraySet<Triple<String, String, Int>> {
val versionCode = PackageInfoCompat.getLongVersionCode(context.packageManager.getPackageInfo(callingPackage, 0))
val requestUrl =
StringBuilder("https://play-fe.googleapis.com/fdfe/delivery?doc=$callingPackage&ot=1&vc=$versionCode&bvc=$versionCode&pf=1&pf=2&pf=3&pf=4&pf=5&pf=7&pf=8&pf=9&pf=10&da=4&bda=4&bf=4&fdcf=1&fdcf=2&ch=")
Copy link
Contributor

@jonathanklee jonathanklee Sep 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried to execute the split installations from TikTok, but I wasn't able to download the split APKs (the url triples were always null) until I changed the URL to:

https://play-fe.googleapis.com/fdfe/delivery?doc=$callingPackage&ot=1&vc=$versionCode

so removing the last part:

&bvc=$versionCode&pf=1&pf=2&pf=3&pf=4&pf=5&pf=7&pf=8&pf=9&pf=10&da=4&bda=4&bf=4&fdcf=1&fdcf=2&ch=

What app did you use to test the split install feature ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When testing with TikTok, I face the following error;

SplitInstallManager: InstallResultReceiver onReceive: install fail -> INSTALL_FAILED_MISSING_SPLIT: Missing split for com.zhiliaoapp.musically

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@DaVinci9196 I was able to install a language split apk on PayPal 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried to execute the split installations from TikTok, but I wasn't able to download the split APKs (the url triples were always null) until I changed the URL to:

https://play-fe.googleapis.com/fdfe/delivery?doc=$callingPackage&ot=1&vc=$versionCode

so removing the last part:

&bvc=$versionCode&pf=1&pf=2&pf=3&pf=4&pf=5&pf=7&pf=8&pf=9&pf=10&da=4&bda=4&bf=4&fdcf=1&fdcf=2&ch=

What app did you use to test the split install feature ?

Tested by switching languages ​​in Google Maps and Bolt

throw IOException("Failed to create directories: ${parentDir.absolutePath}")
}
val fos = FileOutputStream(downloadFile)
fos.write(response.data)
Copy link
Contributor

@fynngodau fynngodau Sep 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Volley loads the entire response into memory. This is a problem for large downloads, as it will consume a lot of RAM (temporarily – or even fail the download if not enough memory is available). The data from the network stream should be written to disk immediately instead.

Per homepage:

Volley is not suitable for large download or streaming operations, since Volley holds all responses in memory during parsing.

Comment on lines +236 to +246
private fun updateSplitInstallRecord(callingPackage: String, triple: Triple<String, String, Int>) {
splitInstallRecord[callingPackage]?.let { triples ->
val find = triples.find { it.first == triple.first }
find?.let { triples.remove(it) }
triples.add(triple)
} ?: run {
val triples = ArraySet<Triple<String, String, Int>>()
triples.add(triple)
splitInstallRecord[callingPackage] = triples
}
}
Copy link
Contributor

@fynngodau fynngodau Sep 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method would be entirely unnecessary if splitInstallRecord had the data type MutableMap<String, Map<PackageComponent, DownloadStatus>> instead of HashMap<String, ArraySet<Triple<String, String, Int>>>, with these new classes:

data class PackageComponent(
    val componentName: String,
    val url: String
)

enum class DownloadStatus {
    PENDING,
    FAILED,
    COMPLETE
}

Or even simpler, a MutableMap<PackageComponent, DownloadStatus> if PackageComponent additionally contained the package's name.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

)
}
NotificationCompat.Builder(context, NOTIFY_CHANNEL_ID).setSmallIcon(android.R.drawable.stat_sys_download)
.setContentTitle(context.getString(R.string.split_install, context.getString(R.string.app_name))).setPriority(NotificationCompat.PRIORITY_DEFAULT)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The placeholder in R.string.split_install should be filled in with the downloading app's name, not with microG's name.

}
}

private fun Context.splitSaveFile() = File(filesDir, FILE_SAVE_PATH)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Installed packages are deleted after installation, but if installation fails and never retried (for instance: one large package is downloaded successfully, then a small package is downloaded unsuccessfully due to a network error, then the user uninstalls the app because it doesn't work), this can still lead to storage leaks from downloaded-but-never-installed apps. We should use cacheDir instead: it will allow the system and users to clear the cache themselves.

putInt(KEY_ERROR_CODE, 0)
putInt(KEY_SESSION_ID, 0)
putLong(KEY_TOTAL_BYTES_TO_DOWNLOAD, intent.getLongExtra(KEY_BYTES_DOWNLOADED, 0))
putString(KEY_LANGUAGES, intent.getStringExtra(KEY_LANGUAGE))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't seem like this intent would have this field, given that it is not set upon creation of the intent in installSplitPackage.

@fynngodau fynngodau mentioned this pull request Sep 22, 2024
11 tasks
@fynngodau
Copy link
Contributor

I opened PR #2553, which contains the code from this PR; I performed some major refactoring to SplitInstallService there.

@mar-v-in mar-v-in removed this from the 0.3.4 milestone Sep 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants